fix: remove defs when ref already defined in schema by cyky · Pull Request #888 · fastify/fastify-swagger

cyky · 2025-07-10T12:08:36Z

When using TypeBox Modules to define Fastify schemas the current implementation duplicates definitions even when using fastify.addSchema(schema). This causes the openapi schema file to be larger and contain duplicate definitions.

If I am correct the OpenApi specification defines that if $ref key is defined for object there shouldn't exist other keys on the object (meaning $defs shouldn't be there). That is why I've added extra check to convertJsonSchemaToOpenapi3to remove $defs-key if $refs-key exits.

I'm open to suggestions if there's a more appropriate way to resolve this.

Usage example with TypeBox modules:

schema.ts

export const HumanModule = Type.Module({
  AddressSchema: Type.Object({
    street: Type.String(),
    streetNumber: Type.Number(),
  }),
  PersonSchema: Type.Object({
    name: Type.String(),
    homeAddress: Type.Ref('AddressSchema'),
    workAddress: Type.Ref('AddressSchema'),
  }),
  PostRequestSchema: Type.Object({
    person: Type.Ref('PersonSchema'),
  }),
});

export const CommonSchema = {
  $id: 'common',
  humanModule: HumanModule,
};

export const PersonSchema = {
  get: {
    response: {
      200: HumanModule.Import('PersonSchema'),
    },
  },
};

index.ts

const fastify = Fastify();

...

app.addSchema(CommonSchema);
app.get(
  '/person',
  {
    schema: {
      response: {
        200: HumanModule.Import('PersonSchema'),
      },
    },
  },
  async (_request, reply) => {
    return reply.status(200);
  }
);

...

await app.ready();
app.swagger();

After the change and using the above style TypeBox module schema definition results in following openapi json:

{
....
  "components": {
    "schemas": {
      "def-0": {
        "humanModule": {
          "$defs": {
            "AddressSchema": {
              "type": "object",
              "properties": {
                "street": { "type": "string" },
                "streetNumber": { "type": "number" }
              },
              "required": ["street", "streetNumber"],
              "title": "AddressSchema"
            },
            "PersonSchema": {
              "type": "object",
              "properties": {
                "name": { "type": "string" },
                "homeAddress": { "$ref": "#/components/schemas/def-1" },
                "workAddress": { "$ref": "#/components/schemas/def-1" }
              },
              "required": ["name", "homeAddress", "workAddress"],
              "title": "PersonSchema"
            },
            "PostRequestSchema": {
              "type": "object",
              "properties": {
                "person": { "$ref": "#/components/schemas/def-2" }
              },
              "required": ["person"],
              "title": "PostRequestSchema"
            }
          }
        },
        "title": "common"
      },
      "def-1": {
        "type": "object",
        "properties": {
          "street": { "type": "string" },
          "streetNumber": { "type": "number" }
        },
        "required": ["street", "streetNumber"],
        "title": "AddressSchema"
      },
      "def-2": {
        "type": "object",
        "properties": {
          "name": { "type": "string" },
          "homeAddress": { "$ref": "#/components/schemas/def-1" },
          "workAddress": { "$ref": "#/components/schemas/def-1" }
        },
        "required": ["name", "homeAddress", "workAddress"],
        "title": "PersonSchema"
      },
      "def-3": {
        "type": "object",
        "properties": { "person": { "$ref": "#/components/schemas/def-2" } },
        "required": ["person"],
        "title": "PostRequestSchema"
      }
    }
  },
  "paths": {
    "/person": {
      "get": {
        "responses": {
          "200": {
            "description": "Default Response",
            "content": {
              "application/json": {
                "schema": { "$ref": "#/components/schemas/def-2" }
              }
            }
          }
        }
      }
    }
  }
  ....
}

This PR should fix issue related to #854

Related issue on TypeBox repository: sinclairzx81/typebox#1283

Checklist

run npm run test && npm run benchmark --if-present
tests and/or benchmarks are included
documentation is changed or added
commit message and code follows the Developer's Certification of Origin
and the Code of conduct

- Remove $defs from schema keys if $ref key already exists when converting json schema into openapi 3. This fixes issue in openapi generation when using TypeBox modules to define schema. - Add test case for refs.

mcollina

Looks correct

Copilot

Pull Request Overview

This PR removes redundant $defs when an object schema already contains a $ref, preventing duplicate definitions in the generated OpenAPI output. It adds a guard in the convertJsonSchemaToOpenapi3 utility to drop $defs when $ref is present and includes a new test to verify this behavior.

Strip $defs when $ref is detected in schema conversion
Add a test case to confirm only the $ref remains and definitions are not duplicated

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/spec/openapi/utils.js	Added a check to delete `$defs` if `$ref` exists in the schema
test/spec/openapi/refs.test.js	New test to validate that only `$ref` is returned when both keys are present

Comments suppressed due to low confidence (1)

test/spec/openapi/refs.test.js:561

Consider adding an assertion that the final component schemas do not contain any $defs keys to ensure the removal logic is fully covered.

  t.assert.deepStrictEqual(openapiObject.paths['/person'].get.responses[200].content, expectedPathContent)

lib/spec/openapi/utils.js

ivan-tymoshenko

Hi, I don't agree with a statement if $ref key is defined for object there shouldn't exist other keys on the object. I would say the correct one is Any sibling elements of a $ref are ignored., so this is still a valid schema.
Why do you remove only $defs then and not all other properties except $ref?
My problem with this change is that it might break some references. For example you can define a schema in the $defs that would have a global id $id: Book. If someone has a ref to this schema $ref: Book it will be broken after this change.

Although the change seems logical, I fear it could break the code in cases where people (including us) don't follow the spec exactly.

cyky · 2025-07-11T07:47:06Z

Seems logical that there should be no siblings for $ref. I could change this to remove all other keys if that follows the spec more closely?

I've created this change because we are using TypeBox to create schemas for our routes and without the change the openapi.json size grows quite large as we have large number of shared types between our routes.

I don't quite understand your how you are using this in a way that doesn't follow OpenAPI specification?

ivan-tymoshenko · 2025-07-11T08:08:10Z

Sorry, I misunderstood the place where the change is done. For the openapi v3 schemas only it should be fine. Please remove all properties to be consistent (not only $defs).

- Remove sibling keys if $refs exits on openapi 3 json schema.

cyky · 2025-07-15T05:08:38Z

I updated the code to remove siblings if $refs is defined.

cyky · 2025-07-22T05:50:22Z

Is it possible to get this merged or are there some steps that need to be done before that?

cyky · 2025-09-01T03:48:25Z

Ping. Would It be possible to get this merged and released?

mcollina

lgtm

joelschou · 2025-11-13T17:33:09Z

Obviously it's too late now, but I just need to say this was an absolutely brutal change to make as part of a patch release. When my package.json has "^9.5.1" I don't expect a fresh clone and install that brings in 9.5.2 to profoundly change the output of my OpenAPI document.

Sure, the result adheres closer to how the spec defines a $ref, but many tools (including Swagger UI!) don't seem to follow those refs properly. For example, the params for a simple GET:

9.5.1

9.5.2

Is that Swagger UI's fault? Of course. Regardless, this stinks.

Eomm · 2025-11-14T07:50:59Z

Are you saying that someone should open an issue at https://github.com/swagger-api/swagger-ui or that there is some other issue on https://github.com/fastify/fastify-swagger-ui ?

In any case I would suggest to create a dedicated issue for that.

Fix: Remove defs when ref already defined

e59c632

- Remove $defs from schema keys if $ref key already exists when converting json schema into openapi 3. This fixes issue in openapi generation when using TypeBox modules to define schema. - Add test case for refs.

Fdawgs linked an issue Jul 10, 2025 that may be closed by this pull request

Invalid references in schema after using TypeBox Modules #854

Closed

2 tasks

Fdawgs changed the title ~~Fix #854: Remove defs when ref already defined in schema~~ fix: remove defs when ref already defined in schema Jul 10, 2025

mcollina approved these changes Jul 11, 2025

View reviewed changes

mcollina requested a review from Copilot July 11, 2025 05:54

Copilot AI reviewed Jul 11, 2025

View reviewed changes

lib/spec/openapi/utils.js Outdated Show resolved Hide resolved

lib/spec/openapi/utils.js Outdated Show resolved Hide resolved

ivan-tymoshenko reviewed Jul 11, 2025

View reviewed changes

Remove sibling keys if $refs exists

2d8d57b

- Remove sibling keys if $refs exits on openapi 3 json schema.

gurgunday approved these changes Jul 20, 2025

View reviewed changes

cyky requested review from gurgunday, ivan-tymoshenko and mcollina September 25, 2025 11:25

mcollina approved these changes Sep 25, 2025

View reviewed changes

gurgunday approved these changes Sep 25, 2025

View reviewed changes

gurgunday merged commit 56dbc02 into fastify:main Sep 25, 2025
14 checks passed

Uh oh!

Conversation

cyky commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

ivan-tymoshenko left a comment

Choose a reason for hiding this comment

Uh oh!

cyky commented Jul 11, 2025

Uh oh!

ivan-tymoshenko commented Jul 11, 2025

Uh oh!

cyky commented Jul 15, 2025

Uh oh!

cyky commented Jul 22, 2025

Uh oh!

cyky commented Sep 1, 2025

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joelschou commented Nov 13, 2025

Uh oh!

Eomm commented Nov 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

cyky commented Jul 10, 2025 •

edited

Loading